<html>
<head><meta charset="utf-8"><title>adding new diagnostic -- stuck implementation · t-compiler/rust-analyzer · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/index.html">t-compiler/rust-analyzer</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html">adding new diagnostic -- stuck implementation</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="244835575"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244835575" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Tedmund Ho <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244835575">(Jul 04 2021 at 02:22)</a>:</h4>
<p>Hi all, sorry if this is the wrong place to post this, but I've been working on this issue <a href="https://github.com/rust-analyzer/rust-analyzer/issues/9457">https://github.com/rust-analyzer/rust-analyzer/issues/9457</a>. (adding of an unnecessary else diagnostic).</p>
<p>I'm slightly stuck on an implementation detail, namely, I'm not sure how to check for a <code>return</code> in the <code>then_branch: ExprId</code> field after destructuring an <code>Expr::If</code> from the body expression. Is there an existing method to extract the body expression of the <code>then_branch</code> or would a better approach be necessary?</p>



<a name="244835714"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244835714" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Tedmund Ho <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244835714">(Jul 04 2021 at 02:27)</a>:</h4>
<p><a href="/user_uploads/4715/tEf9tLiU7B2Ez5J-gP7q5f2N/image.png">image.png</a></p>
<div class="message_inline_image"><a href="/user_uploads/4715/tEf9tLiU7B2Ez5J-gP7q5f2N/image.png" title="image.png"><img src="/user_uploads/4715/tEf9tLiU7B2Ez5J-gP7q5f2N/image.png"></a></div><p>I'm looking at possible changes to this validation function, but am stuck in the actual new <code>check_for_unnecessary_else</code> function I'm trying to implement.</p>



<a name="244851130"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244851130" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244851130">(Jul 04 2021 at 09:40)</a>:</h4>
<p>I <em>think</em> looking at the type of <code>then_branch</code> should help, as it should be <code>!</code> (cc <span class="user-mention" data-user-id="300586">@Lukas Wirth</span> , I think the work you did on tail expressions might intersect with this?)</p>



<a name="244853109"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853109" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Lukas Wirth <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853109">(Jul 04 2021 at 10:38)</a>:</h4>
<p>I don't think looking at the type would work. Since the type of the block is of the last expression and usually that doesn't exist since the tail return is a statement, so that would evaluate to <code>()</code> if I'm not mistaken?</p>



<a name="244853141"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853141" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Lukas Wirth <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853141">(Jul 04 2021 at 10:39)</a>:</h4>
<p>So ye I think the tail walking logic would help out here, basically just walk the tail expression or tail statement if no expression exists and check that all tails returned are returns or breaks basically.</p>



<a name="244853191"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853191" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Lukas Wirth <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853191">(Jul 04 2021 at 10:40)</a>:</h4>
<p>&lt;<a href="https://github.com/rust-analyzer/rust-analyzer/blob/108b56f354f1f5647bdaa229100bef11aae4e464/crates/ide_db/src/helpers.rs#L212-L319">https://github.com/rust-analyzer/rust-analyzer/blob/108b56f354f1f5647bdaa229100bef11aae4e464/crates/ide_db/src/helpers.rs#L212-L319</a>&gt; You can use this to walk the tail expressions, though you will have to move this to some other place as <code>ide_db</code> isnt a dependency of <code>hir_ty</code></p>



<a name="244853207"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853207" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Lukas Wirth <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853207">(Jul 04 2021 at 10:41)</a>:</h4>
<p>That can probably be moved to <code>syntax/src/ast/node_ext.rs</code></p>



<a name="244853563"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853563" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853563">(Jul 04 2021 at 10:51)</a>:</h4>
<p>you shouldn't try to determine whether the block diverges in a diagnostic, we do that during type inference</p>



<a name="244853565"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853565" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853565">(Jul 04 2021 at 10:51)</a>:</h4>
<p>we can add that to the inference result</p>



<a name="244853700"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853700" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853700">(Jul 04 2021 at 10:55)</a>:</h4>
<p>add a new map <a href="https://github.com/rust-analyzer/rust-analyzer/blob/108b56f354f1f5647bdaa229100bef11aae4e464/crates/hir_ty/src/infer.rs#L149">here</a>, move <a href="https://github.com/rust-analyzer/rust-analyzer/blob/108b56f354f1f5647bdaa229100bef11aae4e464/crates/hir_ty/src/infer/expr.rs#L38-L41">this</a> into <code>infer_expr_inner</code> <a href="https://github.com/rust-analyzer/rust-analyzer/blob/108b56f354f1f5647bdaa229100bef11aae4e464/crates/hir_ty/src/infer/expr.rs#L806">here</a>, and write <code>self.diverges</code> into the new map after that in the end of <code>infer_expr_inner</code></p>



<a name="244853814"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853814" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853814">(Jul 04 2021 at 10:58)</a>:</h4>
<p>finally add a <code>expr_diverges</code> (probably just returning bool) analogous to <code>type_of_expr</code> to <code>Semantics</code></p>



<a name="244853907"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244853907" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244853907">(Jul 04 2021 at 11:00)</a>:</h4>
<p>well, if you're implementing it in <code>hir_ty</code> you don't need the <code>Semantics</code> change, though I don't think diagnostics like that (or like <code>ReplaceFilterMapNextWithFindMap</code>) should live there</p>



<a name="244855526"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244855526" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Tedmund Ho <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244855526">(Jul 04 2021 at 11:40)</a>:</h4>
<p><span class="user-mention" data-user-id="129457">@Florian Diebold</span> sorry but could you break down how checking if an expression 'diverges' is relevant for the implementation? Unfortunately I'm not very familiar with the terminologies used, nor seem to be able to find much mention of it in the guide/comments.</p>



<a name="244855557"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244855557" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244855557">(Jul 04 2021 at 11:41)</a>:</h4>
<p>'diverges' means it doesn't reach the end normally, so "the then-block always diverges" is exactly the condition you need for the diagnostic</p>



<a name="244855606"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244855606" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244855606">(Jul 04 2021 at 11:42)</a>:</h4>
<p>(the block could diverge because it has a <code>return</code> statement, but also e.g. because it unconditionally calls <code>panic!</code>)</p>



<a name="244855912"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244855912" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Tedmund Ho <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244855912">(Jul 04 2021 at 11:51)</a>:</h4>
<p>Oh, I see, thanks for the explanation!</p>



<a name="244890211"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244890211" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Tedmund Ho <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244890211">(Jul 05 2021 at 03:14)</a>:</h4>
<p>Hello, I've stumbled upon another slight confusion with regards to some definitions;</p>
<p>In the current type inference implementation, it seems that an <code>Expr::If</code> is evaluated to have diverged if either the condition diverges or <em>both</em> arms diverge. (<a href="https://github.com/rust-analyzer/rust-analyzer/blob/e5c1c8cf2fcfae3e15c8bcf5256e84cad3bd3436/crates/hir_ty/src/infer/expr.rs#L149">https://github.com/rust-analyzer/rust-analyzer/blob/e5c1c8cf2fcfae3e15c8bcf5256e84cad3bd3436/crates/hir_ty/src/infer/expr.rs#L149</a>)</p>
<p>This means that the example in the <a href="https://github.com/rust-analyzer/rust-analyzer/issues/9457">original issue</a> would actually be treated as not having diverged (i.e. <code>Diverges::Maybe</code>)</p>
<div class="codehilite" data-code-language="Rust"><pre><span></span><code><span class="k">fn</span> <span class="nf">test</span><span class="p">()</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">    </span><span class="k">if</span><span class="w"> </span><span class="n">foo</span><span class="w">  </span><span class="p">{</span><span class="w"></span>
<span class="w">        </span><span class="k">return</span><span class="w"> </span><span class="n">bar</span><span class="p">;</span><span class="w"></span>
<span class="w">    </span><span class="p">}</span><span class="w"> </span><span class="k">else</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">        </span><span class="n">do_something_else</span><span class="p">();</span><span class="w"></span>
<span class="w">    </span><span class="p">}</span><span class="w"></span>
<span class="p">}</span><span class="w"></span>
</code></pre></div>
<p>but this would be treated as having diverged (i.e. <code>Diverges::Always</code>)</p>
<div class="codehilite" data-code-language="Rust"><pre><span></span><code><span class="k">fn</span> <span class="nf">test</span><span class="p">()</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">    </span><span class="k">if</span><span class="w"> </span><span class="n">foo</span><span class="w">  </span><span class="p">{</span><span class="w"></span>
<span class="w">        </span><span class="k">return</span><span class="w"> </span><span class="n">bar</span><span class="p">;</span><span class="w"></span>
<span class="w">    </span><span class="p">}</span><span class="w"> </span><span class="k">else</span><span class="w"> </span><span class="p">{</span><span class="w"></span>
<span class="w">        </span><span class="k">return</span><span class="w"> </span><span class="n">do_something_else</span><span class="p">();</span><span class="w"></span>
<span class="w">    </span><span class="p">}</span><span class="w"></span>
<span class="p">}</span><span class="w"></span>
</code></pre></div>
<p>I've written some small tests to verify this and it seems like this may be the case. Is this the intended behaviour?</p>



<a name="244890931"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/adding%20new%20diagnostic%20--%20stuck%20implementation/near/244890931" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Laurențiu <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/adding.20new.20diagnostic.20--.20stuck.20implementation.html#244890931">(Jul 05 2021 at 03:33)</a>:</h4>
<p>That's the whole if. You want to look at the then branch.</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>